Skip to content

Allow for scripts in CKS ISO to fully control CKS deployment #9087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JSpon
Copy link

@JSpon JSpon commented May 16, 2024

Description

This PR...

allows for the CKS cluster to be installed and configured with configuration files in the ISO, thus allowing for more flexibility in the cluster creation. This will allow for the cluster to be built with a custom configuration for kubeadm, as well as packages to be installed using tools like Helm.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

This has been tested in our test environment with custom ISOs that install Cilium instead of Weave, install/upgrade custom Helm charts as part of the deployment, and configure CSI drivers so that CloudStack volumes can be used for persistent volumes.

How did you try to break this feature and the system with this change?

Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.28%. Comparing base (21af134) to head (0d7cc4f).
Report is 52 commits behind head on main.

Current head 0d7cc4f differs from pull request most recent head d1f2c6c

Please upload reports for the commit d1f2c6c to get more accurate results.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9087      +/-   ##
============================================
- Coverage     15.28%   15.28%   -0.01%     
- Complexity    11535    11538       +3     
============================================
  Files          5425     5424       -1     
  Lines        474138   474334     +196     
  Branches      58984    59493     +509     
============================================
+ Hits          72486    72508      +22     
- Misses       393584   393768     +184     
+ Partials       8068     8058      -10     
Flag Coverage Δ
uitests 4.25% <ø> (-0.02%) ⬇️
unittests 16.02% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shwstppr
Copy link
Contributor

@JSpon does this also need a change in create-kubernetes-iso.sh script to include the custom script while preparing the ISO?

@shwstppr
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9638

@weizhouapache
Copy link
Member

I like the idea to setup cks cluster in another way than user-data.
however, as @shwstppr said, there should be some changes with create-kubernetes-iso.sh

can you please also share the setup-kube-system files, for example , to install cilium ? @JSpon
the setup process might be very complicated for end users, to be honest.

@JSpon
Copy link
Author

JSpon commented May 21, 2024

@shwstppr, @weizhouapache - sure. I am testing examples of setup-kube-system (and deploy-kube-system) and will update my PR with the changes to the create-kubernetes-binaries-iso.sh and the examples (and a README.md) after testing is complete

@JSpon JSpon force-pushed the cks-cloud-init-overrides branch from 09f7fe5 to 0a374aa Compare May 23, 2024 18:53
@JSpon
Copy link
Author

JSpon commented May 23, 2024

I made the update to add examples. I had an issue with the first commit, so fixed it and force pushed with the updates.

@weizhouapache
Copy link
Member

I made the update to add examples. I had an issue with the first commit, so fixed it and force pushed with the updates.

thanks @JSpon

it looks like refactoring, almost all codes are copied from the k8s-*.yaml
can you point out the main changes ?

@shwstppr
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9716

@JSpon
Copy link
Author

JSpon commented May 31, 2024

@weizhouapache, the code in basic/scripts is a direct copy, meant as a starting point. The code in cilium has changes only in control-node. The changes are lines 29-35 and 51-63 in setup-kube-system and 31 and 56-57 in deploy-kube-system.tmpl. I can remove the other directories (control-node-add and node) as well as setup-containerd.tmpl in cilium since they aren't needed.

@DaanHoogland
Copy link
Contributor

@blueorangutan test alma9 kvm-alma9

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10329)
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 50281 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9087-t10329-kvm-alma9.zip
Smoke tests completed. 131 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JSpon , sorry to bother you with the apache bylaws, but all files need a license header, except the very trivial ones.

@JSpon
Copy link
Author

JSpon commented Jun 4, 2024

@JSpon , sorry to bother you with the apache bylaws, but all files need a license header, except the very trivial ones.

Sorry about that. Thanks for pointing those out. I have made the updates.

@DaanHoogland DaanHoogland self-requested a review June 4, 2024 18:13
@weizhouapache
Copy link
Member

@JSpon
To be frank, I think we need to solve two problems

  • lots of code are duplicated.
  • lots of parameters are passed to setup-kube-system

I do not have suggestion for now.

I have some simple changes to install cilium by command lines. I will create a draft PR. Maybe we can have more discussions and get a better solution.

I like the idea to run some customized scripts before or after k8s setup.

@JSpon
Copy link
Author

JSpon commented Jun 5, 2024

@weizhouapache, understood. I can also come up with a shell script to extract the information from the existing yml files instead of including copies, and figure a way of easily add cilium into that. This would remove the 18 sample files with all of the duplication in favor of the script, if it is worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants